-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Chore: Package updates #49
Conversation
@@ -12,8 +12,7 @@ | |||
"prepublish": "yarn run lint && yarn run clean && yarn run build", | |||
"test": "jest", | |||
"size": "yarn build && size-limit", | |||
"test:coverage": "jest --coverage", | |||
"report-coverage": "codeclimate-test-reporter < coverage/lcov.info" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This command doesn't work on main
currently and it might never have worked. The package itself has been deprecated. Given that it's not used by anything, I'd rather remove it then "fix" it.
"@size-limit/file": "^8.2.6", | ||
"@size-limit/webpack": "^8.2.6", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@size-limit
switched from defaulting to webpack in v6
to esbuild in v7/v8
. For some reason, this caused our reported build size to nearly quadruple (8.63 kb -> 31.05 kb). I tried to dig into what was going on, but I wasn't able to make much progress.
Given that webpack
is what we were using to determine the size, this feels like less of a breaking change (despite the swap out in packages) and intuitively makes more sense.
@@ -21,31 +21,31 @@ test('flashMessage sets default timeout if none is provided', () => { | |||
const store = createMockStore() | |||
store.dispatch(flashMessage('Hi')) | |||
expect(store.getActionTypes()).toEqual([ADD_MESSAGE_ACTION_TYPE]) | |||
jest.runTimersToTime(4000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method was deprecated
@@ -5,6 +5,8 @@ import { | |||
} from '../src' | |||
import { createMockStore } from './helpers' | |||
|
|||
jest.useFakeTimers() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without this, jest
was reporting a potential unhandled operation (as a result of setTimeout
in middleware.js
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow. I'm sure that was fun to figure out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great Conor!
Resolves #45
Resolves #39 (except for
redux-actions
)I didn't update
redux-actions
to v3 because they switched to ESM only, which our current Jest configuration doesn't understand. I honestly can't be fussed to dig into it further at the minute given how much of a headache ESM/CJS has become.